feat(qwp): support for PEM CA files - #75
Conversation
tls_roots now accepts a PEM certificate file:
QuestDB.connect("wss::addr=db.example.com:9000;tls_roots=/etc/ssl/ca.pem;")
The file may contain one certificate or a bundle, and no longer has to
be imported into a password-protected JKS with keytool first.
Existing JKS/PKCS#12 trust stores keep working as before by supplying
tls_roots_password, and PKCS#12 now also loads on Java 8.
jerrinot
changed the title
Reject tls_roots combined with tls_verify=unsafe_off across Sender and QWP config facades. Enforce the invariant in fluent and low-level TLS construction paths, with regression coverage for PEM and password-protected trust stores.
|
opus-4.8 review: Verification complete. Review: PR #75
|
| Path | Guard | Basis |
|---|---|---|
ILP config fromConfig |
line 3580 | mode |
ILP validateParameters |
line 3981 | mode |
WS static validateWsConfig |
line 3842 | string (only unsafe_off reachable) |
QWP static validateConfig |
line 548 | string (only unsafe_off reachable) |
ClientTlsConfiguration ctor |
line 44 | mode — backstop |
TlsTrustStore.createSslEngine |
line 638 | boolean — runtime backstop |
Programmatic ILP AdvancedTlsSettings |
order-independent cross-guards | — |
Programmatic QWP withTls/withInsecureTls |
clear trustStorePath (1235/1293), "last wins" |
— |
All three transports (tcps→DelegatingTlsChannel, https/wss→JavaTlsClientSocket) route through TlsTrustStore; PEM path keeps endpointIdentificationAlgorithm=https hostname validation. No combination reaches an insecure socket with roots silently dropped — worst case is a build-time exception.
Minor
- PR scope understated. Title says
feat(qwp):but the change equally touches ILP TCP (DelegatingTlsChannel) and ILP HTTP (JavaTlsClientSocket). The first commit already uses the accuratefeat(tls):. Retitle tofeat(tls):so the squash message reflects the real surface. - Inconsistent null-arg messages (nit).
QwpQueryClient.withTrustStore(String)throws"pemRootsPath must not be null"whileAdvancedTlsSettings.customTrustStore(String)null/blank throws"trust store path cannot be empty nor null". Harmless, just inconsistent wording.
Downgraded (checked, not a bug)
- "String-based
unsafe_offguard in the static validators could be bypassed by another spelling." —ConfigView.getEnum(ConfigView.java:140-146) rejects anytls_verifyvalue outside{on, unsafe_off}, case-sensitively, sounsafe_offis the only insecure spelling that reaches the guard, and the mode/boolean backstops catch it even if a static validator ever missed.
Coverage
Solid: TlsTrustStoreTest covers PEM single/bundle/classpath (real checkServerTrusted chain validation), empty-PEM rejection, JKS-without-password → helpful error, and JKS+PKCS12 still-supported. Config-string tests cover the new reject rules on ILP, WS, and QWP. Resources cleaned in finally. assertMemoryLeak not needed (JDK crypto, no native alloc).
Draft findings: 3 raised, 2 minor confirmed, 1 dropped as false positive. Split: 2 in-diff, 0 out-of-diff. Out-of-diff is legitimately zero here — TlsTrustStore is new and its only two callers are both in the diff; the cross-context pass walked all createSslEngine/ClientTlsConfiguration construction sites and found them consistent.
[PR Coverage check]😍 pass : 86 / 100 (86.00%) file detail
|
3 participants
tls_roots now accepts a PEM certificate file:
QuestDB.connect("wss::addr=db.example.com:9000;tls_roots=/etc/ssl/ca.pem;")
The file may contain one certificate or a bundle, and no longer has to be imported into a password-protected JKS with keytool first.
Existing JKS/PKCS#12 trust stores keep working as before by supplying tls_roots_password, and PKCS#12 now also loads on Java 8.